-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] start converting tmpfiles from shell to C #10
base: master
Are you sure you want to change the base?
Conversation
This will make adding a compiled wrapper easier during transition.
We don't compile any code yet. This is just the framework split out for easier review. We'll start compiling things in follow up commits.
For now, we wrap the existing shell script to maintain full compat. We'll start moving over functionality in pieces so we can test.
i'll need to tweak the compiler/CI settings a bit -- looks like Travis defaults to gcc-4.8 which is much too old, and we'll prob need to enable local gtest usage. but neither should matter to the RFC aspects :). |
We don't have any tests here yet, but provides the basic framework for adding them. The test runner can link & run fine though. Tests are written using Google Test (gtest) and in C++ to make our lives easier with more powerful data structures.
These will be used whenever we need to allocate memory and abort if it fails for any reason.
We work with a few sets, so add a data structure for it to avoid ad-hoc implementations in other modules. This is simplify a list as the sets we're working with tends to be quite small: typically less than 10. As long as we don't need to support more, the perf should be good enough.
The shell script is no longer responsible for parsing the command line at all.
This is needed for newer gcc versions.
So far looks good to me :) (I should fight the urge to write it in rust for the fun of it, but given the amount of projects still pending it won't happen anytime soon) |
The first thing I see is I'd rather use meson as a build system instead of makefiles, so should I start the conversion of that on the master branch? |
i'm not keen on rust as i plan on having a shared lib interface. it's prob feasible in rust, but seems like it'll add more complexity to the overall runtime usage ... i have no experience with meson |
Making shared libraries in rust is not hard, and I'm making it even way easier :) If more people have interest we could give it a try, but it can happen at a later time. I would get the C conversion first.
I have some, it is pretty good for our purposes already and looks like gtest has a ready-made recipe for it. |
does anyone have an opinion against using C++ here ? having access to C++ stdlib would help simplify things further (like |
I'd rather do it in rust then. |
yeah, I'm not doing that |
@williamh were you ever able to make progress w/meson here ? or should i just start with makefiles and you can migrate it in parallel ? |
Ok, sorry I've not been able to get to this, a couple of questions: I now have a meson.build on the master branch, but it looks different from what we would need on this branch. How do we go about getting one set up on this branch without affecting the master branch -- @vapier I don't think I can push to your master branch, and I'm not sure that would be a good idea. Have we made any decision about c, c++ or rust? |
I coincidentally picked up rust recently, so I could give it a try. I'm not sure the security aspects matter to the project (as it only ever processes trusted input), but it would include the features that C++ offers over C (i.e. same string handling). |
Sigh, we already have Gentoo people claiming that rust is not an acceptable option because of its packaging and static linking. |
For all we need here, we could even build it using the rustc support provided by meson and no external dependencies. |
Consider the overriding blocker for rust on Gentoo to be keywords: |
i'll move forward with C++ & meson then, and think about a rust thing in parallel @lu-zero i wasn't ignoring your meson PR per se, more waiting on the other questions to be settled. since that seems to be done now, i'll try & pick it back up. |
@vapier it isn't me I think :) On the other hand, C++ has the lovely problem of bringing in If you want to go on with C++ please make sure we can either avoid linking to the C++ runtime at all or we can linking it statically. Adding a rust variant for those that prefer it can happen a later time. |
I think the ABI instability is a bit overstated. it's not really that big of a problem, and if/when it happens again, this project is probably on the very low end of priority. it's not really possible to use the useful C++ features without the runtime. static linking of that has always been possible with a dedicated flag. -static-libstdc++ or something. |
Let see how it goes, probably takes less time to try and see and adapt than keep discussing :) |
Forgot to mention we did https://github.com/rust-torino/tmpfiles-rs/ as part of the meetup activities. |
This is an RFC first off -- I don't intend on merging this directly. I'd like to get feedback about higher level directions before I dive deeper with both documentation and converting code over to compiled code entirely.
For unittesting, I'm writing things in C++ & using gtest. This simplifies test code quite a bit and uses existing common frameworks rather than inventing our own ad-hoc.
I'm assuming people would still prefer to keep the main code in C rather than writing it in C++. Writing it in C++ would simplify resource management, and allow us to use some existing C++ container classes (rather than having to reinvent our own basic ones for C).
Either way, the end result LOC will be higher with the compiled aspect, but it should also be significantly faster, and allow us to address security issues we have now around TOCTOU that are practically impossible to fix in pure shell.